fix(web): Fix loading issues with references / definitions list #617
Conversation
WalkthroughThis PR refactors the API surface by removing domain parameters from search and symbol navigation calls, consolidating API functions into client and server modules, moving type definitions to a dedicated types module, and simplifying authentication flows with a unified optional auth wrapper. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Component
participant ClientAPI as Client API<br/>(packages/web/src/app/api/(client)/client.ts)
participant ServerRoute as Server Route<br/>(e.g., /api/files)
participant BackendAPI as Backend Service
Client->>ClientAPI: getFiles(request) / search(request)
ClientAPI->>ServerRoute: POST to /api/files or /api/search
ServerRoute->>ServerRoute: Parse & Validate Request
alt Validation Failed
ServerRoute-->>ClientAPI: Schema Error Response
else Validation Passed
ServerRoute->>BackendAPI: Call service function<br/>(getFiles, search, etc.)
alt Service Error
BackendAPI-->>ServerRoute: ServiceError
ServerRoute-->>ClientAPI: Error Response
else Service Success
BackendAPI-->>ServerRoute: Result Data
ServerRoute-->>ClientAPI: JSON Response
end
end
ClientAPI-->>Client: Response (Success/Error)
sequenceDiagram
participant UI as UI Components
participant CodeNav as CodeNav Module<br/>(packages/web/src/features/codeNav/api.ts)
participant Auth as Auth Wrapper<br/>(withOptionalAuthV2)
participant BackendSearch as Search Backend
UI->>CodeNav: findSearchBasedSymbolDefinitions(props)
CodeNav->>Auth: Execute with optional auth
Auth->>CodeNav: Proceed (auth resolved or optional)
CodeNav->>BackendSearch: Query for symbol definitions
BackendSearch-->>CodeNav: Search Results
CodeNav->>CodeNav: Parse Results<br/>(parseRelatedSymbolsSearchResponse)
CodeNav-->>UI: Response | ServiceError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (2)
43-50: Removedomainfrom the queryKey.The query key includes
domainon line 43, but the API functionfindSearchBasedSymbolReferencesno longer accepts or uses the domain parameter. Including it in the queryKey could cause unnecessary cache invalidations and re-fetches when the domain changes, even though the API response wouldn't actually differ.Apply this diff:
- queryKey: ["references", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language, domain], + queryKey: ["references", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language],
59-66: Removedomainfrom the queryKey.Similar to the references query, the query key includes
domainon line 59, butfindSearchBasedSymbolDefinitionsno longer accepts or uses the domain parameter.Apply this diff:
- queryKey: ["definitions", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language, domain], + queryKey: ["definitions", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language],
🧹 Nitpick comments (9)
packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx (1)
3-107: File source fetching is correctly migrated to the new client APIImporting
getFileSourcefrom"@/app/api/(client)/client"and invoking it viaunwrapServiceError(getFileSource({ fileName, repository, branch }))aligns this query with the new REST-backed client API and removes the old server action dependency while still allowing parallel loading viauseQueries.One minor follow-up you might consider (not required for this PR): since
unwrapServiceErroralready throws onServiceError, the laterisServiceError(query.data)check will never see aServiceErrorvalue; consolidating on one error-handling pattern here would simplify the code.packages/web/src/app/api/(client)/client.ts (4)
72-78: Add Content-Type header for consistency.The new API functions should include the
Content-Type: application/jsonheader in their fetch requests for consistency with the existingsearchandgetFileSourcefunctions (lines 27 and 43). While browsers may infer the content type, explicit headers are a best practice.Apply this diff:
export const findSearchBasedSymbolReferences = async (body: FindRelatedSymbolsRequest): Promise<FindRelatedSymbolsResponse | ServiceError> => { const result = await fetch("/api/find_references", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as FindRelatedSymbolsResponse | ServiceError; }
80-86: Add Content-Type header for consistency.Same issue as the previous function.
Apply this diff:
export const findSearchBasedSymbolDefinitions = async (body: FindRelatedSymbolsRequest): Promise<FindRelatedSymbolsResponse | ServiceError> => { const result = await fetch("/api/find_definitions", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as FindRelatedSymbolsResponse | ServiceError; }
88-94: Add Content-Type header for consistency.Same issue as the previous functions.
Apply this diff:
export const getTree = async (body: GetTreeRequest): Promise<GetTreeResponse | ServiceError> => { const result = await fetch("/api/tree", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as GetTreeResponse | ServiceError; }
96-102: Add Content-Type header for consistency.Same issue as the previous functions.
Apply this diff:
export const getFiles = async (body: GetFilesRequest): Promise<GetFilesResponse | ServiceError> => { const result = await fetch("/api/files", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as GetFilesResponse | ServiceError; }packages/web/src/features/fileTree/api.ts (1)
196-235: Type annotations are good, butpathfor intermediate nodes is likely incorrectThe added
FileTreeNodeannotations in the.find(Line 212) and.sort(Line 235) callbacks are correct and align with the exported schema type.One thing to watch in this function: for intermediate directory nodes you’re still setting
path: item.path,(Line 217), which means every ancestor directory of a file ends up with the full leaf path rather than its own segment path (e.g.
srcnode getssrc/pages/index.tsx). If any consumer relies on a directory node’spathbeing its actual directory path, this will be misleading.Consider computing the path from the current prefix instead, e.g.:
const nodePath = parts.slice(0, i + 1).join("/"); ... path: nodePath,and keeping
item.pathonly for the leaf.packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
52-77: Updated call signature is correct; minor note on domain dependencyUsing
findSearchBasedSymbolDefinitions({ symbolName: symbolName!, language, revisionName, })(Line 55–59) matches the new “single props object” API and works fine with
enabled: !!symbolName, so the non‑null assertion is safe in practice.
domainis still part of thequeryKeyand theuseEffectdependency list but no longer part of the payload. That’s OK for cache key separation and re‑binding listeners, though thedomaindependency in theuseEffectbody is now unused and could be dropped later for clarity.packages/web/src/features/fileTree/types.ts (1)
1-44: Recursive file tree schemas look sound; consider tightening node typeThe zod setup for
FileTreeItem,FileTreeNode, and the request/response shapes is structurally correct, and the recursivefileTreeNodeSchemausingz.lazyis idiomatic.If you know the
typefield is constrained (e.g."blob" | "tree"plus maybe"commit"for submodules), you could optionally tightentype: z.string(),to a
z.enum([...])(and mirror that inFileTreeNodeType) to catch invalid data earlier. Not required for this PR, but would strengthen the contract between the git parsing code and consumers.packages/web/src/features/codeNav/api.ts (1)
39-61: findSearchBasedSymbolDefinitions matches the references helper; consider deduping the shared logicThe definitions helper mirrors the references version with the expected query change:
const query = `sym:\\b${symbolName}\\b rev:${revisionName} ${getExpandedLanguageFilter(language)}`;and the same error handling + parsing flow. This is exactly what you want for consistency between refs/defs.
Since both functions now share nearly identical control flow, you could optionally pull out a small internal helper that takes the query template (or a mode enum) to reduce duplication, but that’s not required for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
CHANGELOG.md(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx(1 hunks)packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts(2 hunks)packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPage.tsx(1 hunks)packages/web/src/app/api/(client)/client.ts(3 hunks)packages/web/src/app/api/(server)/files/route.ts(1 hunks)packages/web/src/app/api/(server)/find_definitions/route.ts(1 hunks)packages/web/src/app/api/(server)/find_references/route.ts(1 hunks)packages/web/src/app/api/(server)/tree/route.ts(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx(3 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts(2 hunks)packages/web/src/features/chat/agent.ts(0 hunks)packages/web/src/features/chat/components/chatBox/useSuggestionsData.ts(1 hunks)packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx(2 hunks)packages/web/src/features/chat/tools.ts(3 hunks)packages/web/src/features/codeNav/api.ts(2 hunks)packages/web/src/features/codeNav/schemas.ts(0 hunks)packages/web/src/features/codeNav/types.ts(1 hunks)packages/web/src/features/fileTree/api.ts(4 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx(1 hunks)packages/web/src/features/fileTree/components/pureFileTreePanel.tsx(1 hunks)packages/web/src/features/fileTree/types.ts(1 hunks)packages/web/src/features/search/fileSourceApi.ts(1 hunks)packages/web/src/features/search/searchApi.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web/src/features/chat/agent.ts
- packages/web/src/features/codeNav/schemas.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsxCHANGELOG.mdpackages/web/src/app/api/(server)/find_definitions/route.tspackages/web/src/features/search/searchApi.tspackages/web/src/app/api/(server)/find_references/route.tspackages/web/src/features/fileTree/components/fileTreeItemIcon.tsxpackages/web/src/app/api/(server)/files/route.tspackages/web/src/app/[domain]/components/searchBar/useSuggestionsData.tspackages/web/src/features/fileTree/components/fileTreePanel.tsxpackages/web/src/features/fileTree/api.tspackages/web/src/features/chat/tools.tspackages/web/src/features/codeNav/types.tspackages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsxpackages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsxpackages/web/src/features/codeNav/api.tspackages/web/src/features/fileTree/types.tspackages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.tspackages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsxpackages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsxpackages/web/src/app/api/(server)/tree/route.tspackages/web/src/features/fileTree/components/pureFileTreePanel.tsxpackages/web/src/features/fileTree/components/fileTreeItemComponent.tsxpackages/web/src/ee/features/codeNav/components/exploreMenu/index.tsxpackages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsxpackages/web/src/features/chat/components/chatBox/useSuggestionsData.tspackages/web/src/app/api/(client)/client.tspackages/web/src/features/search/fileSourceApi.tspackages/web/src/app/[domain]/search/components/searchResultsPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (33)
packages/web/src/features/search/searchApi.ts (3)
1-1: Confirm 'server-only' import is the correct Next.js 15 migration pattern.Replacing the "use server" directive with an explicit 'server-only' import aligns with Next.js 15's shift toward structured runtime marking. Verify that this module is never imported by client components and that all consumers are aware of the server-only context.
64-74: Verify Prisma context is available at this point in the async flow.The
transformZoektQueryfunction receivesprismaas a parameter and makes queries againstprisma.searchContext. Confirm thatprismais properly injected bywithOptionalAuthV2before this function is called, especially in the Next.js 15 async context where database access patterns may have changed.
130-131: Wrapper chain correctly implements async auth context handling.Verification confirms all three integration points are working correctly:
✓
sewunwrapping: The wrapper correctly receives() => Promise<T>fromwithOptionalAuthV2(async { org, prisma } => {...}), awaits it, and returnsPromise<T | ServiceError>.✓ Prisma injection:
withOptionalAuthV2properly injectsorg,prisma,user, androleinto the callback context, enabling Prisma queries likesearchContext.findUnique()used in the search logic.✓ Consumer code updated: All identified call sites (codeNav/api.ts, chat/tools.ts, fileSourceApi.ts, useSuggestionsData.ts, searchResultsPage.tsx, search/route.ts) have been updated and correctly pass only
{ query, matches, contextLines, whole }without domain parameters.CHANGELOG.md (1)
10-12: LGTM!The changelog entry clearly documents the fix and follows the Keep a Changelog format.
packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts (2)
54-58: LGTM!The removal of the
domainparameter from thesearch()call aligns with the PR's objective to migrate from server actions to REST endpoints. The domain is still appropriately retained in thequeryKeyfor proper cache management.
74-78: LGTM!Consistent removal of the
domainparameter from the symbol search call, matching the file search changes above.packages/web/src/features/chat/tools.ts (3)
6-6: LGTM!The import path update from
../codeNav/actionsto../codeNav/apireflects the API surface reorganization described in the PR.
34-38: LGTM!The removal of the domain parameter simplifies the API call and aligns with the migration to REST endpoints that handle domain context implicitly.
71-75: LGTM!Consistent with the symbol references changes above—the domain parameter has been appropriately removed.
packages/web/src/app/api/(server)/files/route.ts (1)
1-23: LGTM!The new server route handler follows a clean pattern with:
- Proper schema validation using
getFilesRequestSchema- Appropriate error handling via
serviceErrorResponse- Consistent structure with other server routes in this PR
This aligns with the PR's goal of migrating from server actions to REST endpoints to resolve loading issues.
packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)
9-9: LGTM!The import consolidation into the client API module aligns with the PR's API surface reorganization.
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
7-7: LGTM!The import reordering is a benign organizational change with no functional impact.
packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (1)
3-3: LGTM!Moving the
RawFileTreeNodetype import to the dedicatedtypesmodule improves code organization and aligns with the PR's restructuring goals.packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1)
6-6: LGTM!Consistent with the type organization improvements—moving
FileTreeItemto the centralized types module.packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1)
3-23: File tree import migration looks correctSwitching
getFolderContentsto the centralized"@/features/fileTree/api"import while keeping the existing payload andPromise.allusage intact is consistent with the new file tree API surface and should preserve behavior.packages/web/src/features/search/fileSourceApi.ts (1)
1-61: Marking this module as server-only is appropriateUsing a bare
import 'server-only';to enforce thatgetFileSourceonly runs on the server aligns with the intended usage of this API and replaces the old'use server'directive without changing behavior.Please confirm this project is on a Next.js version that officially supports the
server-onlypackage for server-only module guards.packages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsx (1)
3-13: FileTreeItem type import now correctly sourced from types modulePointing
FileTreeItemat"@/features/fileTree/types"matches the new shared types module and keeps this component aligned with the rest of the file tree API.packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
3-23: Consistent use of shared FileTreeItem typeUpdating
FileTreeItemto come from../typeskeeps this component in sync with the centralized file tree type definitions without affecting runtime behavior.packages/web/src/features/chat/components/chatBox/useSuggestionsData.ts (1)
32-45: Search call updated to match new payload-only APIThe
searchinvocation in the file suggestions query now correctly uses a single payload object ({ query, matches, contextLines }) without a separate domain parameter, which matches the refactored client search API. Keepingdomainin the query key for cache scoping also still makes sense.packages/web/src/app/[domain]/search/components/searchResultsPage.tsx (1)
57-70: SearchResultsPage now uses the unified search payloadAdjusting the
searchcall in the main query function to pass only{ query, matches, contextLines, whole }(no domain argument) matches the new client search contract while preserving the existingmeasureandunwrapServiceErrorbehavior.packages/web/src/app/api/(server)/tree/route.ts (1)
1-22: LGTM! Well-structured server route handler.The implementation follows the correct pattern with proper validation, error handling, and response formatting. The use of
Response.json()automatically sets the Content-Type header appropriately.packages/web/src/app/api/(server)/find_definitions/route.ts (1)
1-22: LGTM! Consistent implementation pattern.The route handler follows the same well-structured pattern as the other server routes in this PR, with proper schema validation and error handling.
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1)
15-16: LGTM! Proper import reorganization.The imports have been correctly updated to use the centralized client API module and shared types, improving code organization.
packages/web/src/app/api/(server)/find_references/route.ts (2)
7-20: LGTM! Route handler implementation is correct.Once the 'use server' directive is added, the route handler implementation follows the correct pattern with proper validation and error handling.
1-5: Missing 'use server' directive.This file is missing the
'use server'directive at the top that is present in the other server route files (tree/route.ts line 1, find_definitions/route.ts line 1). Without this directive, the file may not be properly treated as server-only code by Next.js.Apply this diff:
+'use server'; + import { findSearchBasedSymbolReferences } from "@/features/codeNav/api"; import { findRelatedSymbolsRequestSchema } from "@/features/codeNav/types"; import { schemaValidationError, serviceErrorResponse } from "@/lib/serviceError"; import { isServiceError } from "@/lib/utils"; import { NextRequest } from "next/server";Likely an incorrect or invalid review comment.
packages/web/src/features/codeNav/types.ts (1)
1-29: LGTM! Well-structured type definitions.The schema definitions are comprehensive and properly structured, with appropriate use of Zod for validation and type inference. The request and response schemas are well-typed and will provide good type safety throughout the application.
packages/web/src/features/fileTree/components/fileTreePanel.tsx (2)
45-53: LGTM! Correct API integration.The file tree data fetching has been properly updated to use the new client API with appropriate error handling via
unwrapServiceError.
55-65: LGTM! Good UX enhancement.The keyboard shortcut implementation provides a nice user experience for toggling the file tree panel.
packages/web/src/features/fileTree/api.ts (1)
1-11: Shared file tree types import looks consistentUsing
FileTreeItem/FileTreeNodefrom./typeskeeps this API in sync with the new zod-backed schemas and avoids divergent local shapes. No issues here.packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)
1-1: Client API import aligns with new symbol definitions endpointSwitching
findSearchBasedSymbolDefinitionsto the client module (@/app/api/(client)/client) matches the refactor away from server actions and keeps this hook on the new API surface. No issues here.packages/web/src/features/codeNav/api.ts (3)
1-11: Auth wrapper + type imports wired correctly for the new API surfaceUsing
'server-only'plussewandwithOptionalAuthV2keeps these helpers firmly on the server and aligns with the rest of the codebase’s service-error handling. ImportingFindRelatedSymbolsRequest/Responsefrom./typescentralizes the payload/response shapes, which matches the PR’s goal of consolidating API types.
15-37: findSearchBasedSymbolReferences: props-based signature and error handling look goodThe new signature
export const findSearchBasedSymbolReferences = async ( props: FindRelatedSymbolsRequest ): Promise<FindRelatedSymbolsResponse | ServiceError> => sew(() => withOptionalAuthV2(async () => { ... }) );correctly removes the
domainparameter and relies on a single props object. The query string (\b${symbolName}\b rev:${revisionName} ... case:yes) mirrors previous behavior, andisServiceError(searchResult)ensures you don’t feed errors into the parser.No functional issues here; the layering (auth → search → parse) is clean.
62-88: parseRelatedSymbolsSearchResponse correctly flattens the search result structureThe parser built from
searchResponseSchema.transform(async ({ files }) => ...)andparser.parseAsync(searchResult):
- Preserves
stats.actualMatchCountandrepositoryInfofrom the originalsearchResult.- Flattens
file.chunks[*].matchRanges[*]into amatchesarray withlineContentandrange.- Filters out files with zero matches.
This matches the expected
FindRelatedSymbolsResponseshape and keeps parsing logic centralized. Looks good.
brendan-kellam
deleted the
bkellam/migrate_queries_from_action_to_api
branch
We were getting spurious loading issues with the references / definitions panel and file tree. The root cause of this was we were using server actions which cannot run in parallel and are ill-suited for client side queries. This PR migrates a couple of actions over to REST endpoints.
Summary by CodeRabbit
Release Notes